-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(wrangler/cli): add @bomb.sh/tab completions
#11113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: b2fc39e The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
45886d3 to
e9c0d42
Compare
dario-piotrowicz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome stuff, thanks so much @AmirSa12 🙏
I've left a comment regarding the completions setup, could you take a look? 😄
Also could you advise on how I can manually test this feature? 🙂
(is there a setup required, etc...)
packages/wrangler/src/complete.ts
Outdated
| @@ -0,0 +1,304 @@ | |||
| // NOTE: The provided option-value completions can be customized or removed as needed. | |||
| // NOTE: Descriptions and flags based on https://developers.cloudflare.com/workers/wrangler/commands/ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that taking the values from the docs and using them here is a bit problematic (as we'd also need to keep them in sync, etc...)
Could you try to instead use the experimental_getWranglerCommands function to dynamically get the information you need and setup the completions from that? 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @dario-piotrowicz ! Thanks for your review!
Yes, I will try to use the experimental_getWranglerCommands and let you know how it works with that!
I'll keep you posted!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! thanks! 🫶
Thank you, @dario-piotrowicz! on top of this, if you have your own cli, you can have completions for that as well, and if your cli is executed through a package manager, and supports tab completions, lets assume wrangler does, the next example would also work: |
Thanks @AmirSa12 😄 So... if I understand correctly for |
@dario-piotrowicz, the whole tab package relies on shell scripts.. all the magic happens in the shell, so in-order to have the completions, we need a shell script ( think of it as a plugin ) to inject in our terminal. by installing tab, you are only injecting the shell script, in other ways this means you can directly do |
I see, great! thanks for the clarification 😄🙏 |
|
@dario-piotrowicz , I'm amazed by what I could achieve via the but one note, I noticed that, by using this function, we are only handling options that have explicit choices defined in the command args (like log-level) some option values (like |
|
Hey! I'd like to add some context and technical details regarding the approaches used in both this PR and #11637 On top of this, from a quality and reliability standpoint, tests can be added for this PR as well.
If this approach is still not desired, I recommend an alternative to be the cli tool wrangler is using (yargs) as it provides completions. however it’s important to note that Additionally, we're happy to add tests or address any missing features or gaps you think cc @dario-piotrowicz @NuroDev @ascorbic @dmmulroy wrangler.mp4 |
|
Hey @AmirSa12, is there anything else needed on this or do you need any help with anything? Happy to lend a hand to help get this over the line and merged 😄 |
|
hey @NuroDev, sorry for the inactivity, we've had some personal issues on the side, i'll look into this and see if i can move it forward! |
|
ok i had a look, i think as you suggested, maybe we can merge this PR and open another PR later that can add more It might take more work so splitting it up in another PR might be a quicker and cleaner way to do this. This way when @AmirSa12 is back, he can have a serious look at it too. Let me know what you think @NuroDev, and again, really sorry for the delay! |
NuroDev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just added a few tiny nit pick suggestions about removing some self explanitory comments, and 2 small comments.
Apart from that this LGTM and ready to go 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
thank you so much team for this, we really appreciate it. |
This PR introduces tab completion functionality for the wrangler cli, improving developer experience by allowing shell(zsh, powershell, fish, bash) autocompletion for commands, options, values, and flags. This also comes with npm, pnpm, yarn and bun copmpletions! ( npm exec ... , pnpm ... ) With this PR, users can navigate available commands, options and values more efficiently and faster with speeding up workflow.
A small video of how this looks:
wrangler.mp4
wranglercloudflare-docs#27514Fixes #53